Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update color palette popover style #41900

Merged
merged 10 commits into from
Jul 26, 2022
Merged

Conversation

jameskoster
Copy link
Contributor

@jameskoster jameskoster commented Jun 23, 2022

Before After
Screenshot 2022-06-23 at 10 34 22 Screenshot 2022-06-23 at 10 34 14

This PR proposes some small style adjustments that I think improve the appearance of the palette popover.

Changes

  • Increased padding. On trunk the gap between swatches is larger than the popover padding which creates some visual tension in my eye.
  • Popover width is calculated mathematically to ensure it wraps 6 swatches snugly.
  • Removed a seemingly random bottom margin on the popover contents.

Still todo

  • Apply these changes to the palette popover accessed via the border control which uses a different implementation.

@jameskoster jameskoster added Needs Design Feedback Needs general design feedback. [Feature] Colors Color management labels Jun 23, 2022
@jameskoster jameskoster requested a review from a team June 23, 2022 09:41
@github-actions
Copy link

github-actions bot commented Jun 23, 2022

Size Change: +95 B (0%)

Total Size: 1.26 MB

Filename Size Change
build/block-editor/index.min.js 153 kB +99 B (0%)
build/block-editor/style-rtl.css 14.6 kB -2 B (0%)
build/block-editor/style.css 14.6 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 982 B
build/annotations/index.min.js 2.76 kB
build/api-fetch/index.min.js 2.26 kB
build/autop/index.min.js 2.14 kB
build/blob/index.min.js 475 B
build/block-directory/index.min.js 6.58 kB
build/block-directory/style-rtl.css 990 B
build/block-directory/style.css 991 B
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 103 B
build/block-library/blocks/audio/style.css 103 B
build/block-library/blocks/audio/theme-rtl.css 110 B
build/block-library/blocks/audio/theme.css 110 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 59 B
build/block-library/blocks/avatar/style.css 59 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 441 B
build/block-library/blocks/button/editor.css 441 B
build/block-library/blocks/button/style-rtl.css 542 B
build/block-library/blocks/button/style.css 542 B
build/block-library/blocks/buttons/editor-rtl.css 292 B
build/block-library/blocks/buttons/editor.css 292 B
build/block-library/blocks/buttons/style-rtl.css 275 B
build/block-library/blocks/buttons/style.css 275 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 103 B
build/block-library/blocks/code/style.css 103 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 187 B
build/block-library/blocks/comment-template/style.css 185 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 834 B
build/block-library/blocks/comments/editor.css 832 B
build/block-library/blocks/comments/style-rtl.css 632 B
build/block-library/blocks/comments/style.css 630 B
build/block-library/blocks/cover/editor-rtl.css 615 B
build/block-library/blocks/cover/editor.css 616 B
build/block-library/blocks/cover/style-rtl.css 1.55 kB
build/block-library/blocks/cover/style.css 1.55 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 110 B
build/block-library/blocks/embed/theme.css 110 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 253 B
build/block-library/blocks/file/style.css 254 B
build/block-library/blocks/file/view.min.js 346 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 948 B
build/block-library/blocks/gallery/editor.css 950 B
build/block-library/blocks/gallery/style-rtl.css 1.53 kB
build/block-library/blocks/gallery/style.css 1.53 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 333 B
build/block-library/blocks/group/editor.css 333 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 327 B
build/block-library/blocks/html/editor.css 329 B
build/block-library/blocks/image/editor-rtl.css 736 B
build/block-library/blocks/image/editor.css 737 B
build/block-library/blocks/image/style-rtl.css 627 B
build/block-library/blocks/image/style.css 630 B
build/block-library/blocks/image/theme-rtl.css 110 B
build/block-library/blocks/image/theme.css 110 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 199 B
build/block-library/blocks/latest-posts/editor.css 198 B
build/block-library/blocks/latest-posts/style-rtl.css 463 B
build/block-library/blocks/latest-posts/style.css 462 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 493 B
build/block-library/blocks/media-text/style.css 490 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 705 B
build/block-library/blocks/navigation-link/editor.css 703 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation-submenu/view.min.js 423 B
build/block-library/blocks/navigation/editor-rtl.css 2.03 kB
build/block-library/blocks/navigation/editor.css 2.04 kB
build/block-library/blocks/navigation/style-rtl.css 1.96 kB
build/block-library/blocks/navigation/style.css 1.95 kB
build/block-library/blocks/navigation/view-modal.min.js 2.78 kB
build/block-library/blocks/navigation/view.min.js 443 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 363 B
build/block-library/blocks/page-list/editor.css 363 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 260 B
build/block-library/blocks/paragraph/style.css 260 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 493 B
build/block-library/blocks/post-comments-form/style.css 493 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 605 B
build/block-library/blocks/post-featured-image/editor.css 605 B
build/block-library/blocks/post-featured-image/style-rtl.css 153 B
build/block-library/blocks/post-featured-image/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 282 B
build/block-library/blocks/post-template/style.css 282 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 80 B
build/block-library/blocks/post-title/style.css 80 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 370 B
build/block-library/blocks/pullquote/style.css 370 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 234 B
build/block-library/blocks/query-pagination/style.css 231 B
build/block-library/blocks/query/editor-rtl.css 365 B
build/block-library/blocks/query/editor.css 364 B
build/block-library/blocks/quote/style-rtl.css 213 B
build/block-library/blocks/quote/style.css 213 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 385 B
build/block-library/blocks/search/style.css 386 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 233 B
build/block-library/blocks/separator/style.css 233 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 464 B
build/block-library/blocks/shortcode/editor.css 464 B
build/block-library/blocks/site-logo/editor-rtl.css 708 B
build/block-library/blocks/site-logo/editor.css 708 B
build/block-library/blocks/site-logo/style-rtl.css 192 B
build/block-library/blocks/site-logo/style.css 192 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.39 kB
build/block-library/blocks/social-links/style.css 1.38 kB
build/block-library/blocks/spacer/editor-rtl.css 322 B
build/block-library/blocks/spacer/editor.css 322 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 494 B
build/block-library/blocks/table/editor.css 494 B
build/block-library/blocks/table/style-rtl.css 611 B
build/block-library/blocks/table/style.css 609 B
build/block-library/blocks/table/theme-rtl.css 175 B
build/block-library/blocks/table/theme.css 175 B
build/block-library/blocks/tag-cloud/style-rtl.css 226 B
build/block-library/blocks/tag-cloud/style.css 227 B
build/block-library/blocks/template-part/editor-rtl.css 235 B
build/block-library/blocks/template-part/editor.css 235 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 561 B
build/block-library/blocks/video/editor.css 563 B
build/block-library/blocks/video/style-rtl.css 159 B
build/block-library/blocks/video/style.css 159 B
build/block-library/blocks/video/theme-rtl.css 110 B
build/block-library/blocks/video/theme.css 110 B
build/block-library/common-rtl.css 1.01 kB
build/block-library/common.css 1 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 10.9 kB
build/block-library/editor.css 10.9 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/index.min.js 184 kB
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/style-rtl.css 11.8 kB
build/block-library/style.css 11.8 kB
build/block-library/theme-rtl.css 695 B
build/block-library/theme.css 700 B
build/block-serialization-default-parser/index.min.js 1.11 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/blocks/index.min.js 47.2 kB
build/components/index.min.js 230 kB
build/components/style-rtl.css 14 kB
build/components/style.css 14 kB
build/compose/index.min.js 11.7 kB
build/core-data/index.min.js 14.7 kB
build/customize-widgets/index.min.js 11.2 kB
build/customize-widgets/style-rtl.css 1.4 kB
build/customize-widgets/style.css 1.4 kB
build/data-controls/index.min.js 653 B
build/data/index.min.js 7.99 kB
build/date/index.min.js 32 kB
build/deprecated/index.min.js 507 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.69 kB
build/edit-navigation/index.min.js 16 kB
build/edit-navigation/style-rtl.css 4.02 kB
build/edit-navigation/style.css 4.03 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/edit-post/index.min.js 30.5 kB
build/edit-post/style-rtl.css 6.94 kB
build/edit-post/style.css 6.94 kB
build/edit-site/index.min.js 54.4 kB
build/edit-site/style-rtl.css 8.18 kB
build/edit-site/style.css 8.16 kB
build/edit-widgets/index.min.js 16.5 kB
build/edit-widgets/style-rtl.css 4.35 kB
build/edit-widgets/style.css 4.35 kB
build/editor/index.min.js 41.3 kB
build/editor/style-rtl.css 3.66 kB
build/editor/style.css 3.65 kB
build/element/index.min.js 4.27 kB
build/escape-html/index.min.js 537 B
build/format-library/index.min.js 6.75 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.64 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.77 kB
build/is-shallow-equal/index.min.js 527 B
build/keyboard-shortcuts/index.min.js 1.78 kB
build/keycodes/index.min.js 1.38 kB
build/list-reusable-blocks/index.min.js 1.74 kB
build/list-reusable-blocks/style-rtl.css 835 B
build/list-reusable-blocks/style.css 835 B
build/media-utils/index.min.js 2.93 kB
build/notices/index.min.js 953 B
build/nux/index.min.js 2.05 kB
build/nux/style-rtl.css 732 B
build/nux/style.css 728 B
build/plugins/index.min.js 1.94 kB
build/preferences-persistence/index.min.js 2.22 kB
build/preferences/index.min.js 1.3 kB
build/primitives/index.min.js 933 B
build/priority-queue/index.min.js 612 B
build/react-i18n/index.min.js 696 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.68 kB
build/reusable-blocks/index.min.js 2.22 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 11.1 kB
build/server-side-render/index.min.js 1.61 kB
build/shortcode/index.min.js 1.53 kB
build/token-list/index.min.js 644 B
build/url/index.min.js 3.61 kB
build/vendors/react-dom.min.js 38.5 kB
build/vendors/react.min.js 4.34 kB
build/viewport/index.min.js 1.08 kB
build/warning/index.min.js 268 B
build/widgets/index.min.js 7.19 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.06 kB

compressed-size-action

@javierarce javierarce added the Needs Figma Update Needs an update to Figma for design purposes label Jun 23, 2022
@javierarce
Copy link
Contributor

Nice! I've just tested it and it looks good to me ✨

@paaljoachim
Copy link
Contributor

I agree. The changes looks good to me as well!

@jameskoster
Copy link
Contributor Author

cc @ciampo @mirka about how to proceed here. Repeating the work specifically for the color picker in the border control doesn't seem like the optimal approach.

@ciampo
Copy link
Contributor

ciampo commented Jul 19, 2022

We could try to implement the solution proposed in #41937 (comment) — what do you think, @mirka ?

@mirka
Copy link
Member

mirka commented Jul 19, 2022

Yes, I'll look into this within the next few days and report back 👍

@mirka
Copy link
Member

mirka commented Jul 21, 2022

Progress update

  • Padding: I'm proposing a solution in Dropdown: Add wrapper for custom padding #42595.
  • Weird bottom margin: This can actually be removed via a prop on BaseControl. (We're actually aiming to deprecate this margin.)
  • Content width: I'm kind of indifferent about this at the moment. If it's just for color-gradients and BorderControl, it might be too early to abstract into some kind of shared system. I don't mind keeping this ad hoc approach until we see one or two more instances where this is needed.

@mirka mirka force-pushed the update/color-palette-popover branch from ce18f29 to a190d78 Compare July 22, 2022 16:40
@mirka
Copy link
Member

mirka commented Jul 22, 2022

@jameskoster Ok I'm done ✅

  • Increased padding is now applied in a clean way.

  • Width is set without reaching into the internal .components-popover__content class.

  • Same styling changes are applied on BorderControl:

    CleanShot.2022-07-23.at.01.39.05.mp4

☝️ By the way, do we care that the checkered pattern for the "transparent" color is now cut off at an awkward place? The background pattern isn't adhering to the 4px grid system (the squares are 25px). We might want to address that in a follow-up.

@jameskoster
Copy link
Contributor Author

Thanks @mirka ! :)

The padding is a bit awkward here since we switched to tabs:

Screenshot 2022-07-25 at 10 14 39

But I think that's something to fix in the tabpanel component (#42320).

The background pattern isn't adhering to the 4px grid system (the squares are 25px). We might want to address that in a follow-up.

Agreed. For now I don't think it's a deal-breaker.

Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think that's something to fix in the tabpanel component (#42320).

Good call. We can figure out a manageable API once we have the design spec.

I split out the transparent checkered background issue to #42687.

Code-wise, this PR is good to go 👍

@paaljoachim
Copy link
Contributor

paaljoachim commented Jul 26, 2022

Sidetrack comment...Related to the Color Panels.
It seems like the Link Color Panel has a Clear color text option while the Text and Background color options do not.
All Color Panels should have the clear option.
Made an issue for it here: #42689

@jameskoster jameskoster merged commit 2f1e1dc into trunk Jul 26, 2022
@jameskoster jameskoster deleted the update/color-palette-popover branch July 26, 2022 11:35
@github-actions github-actions bot added this to the Gutenberg 13.8 milestone Jul 26, 2022
@jasmussen
Copy link
Contributor

Coming back to this one (thank you for the energy and attention to details!) — I like the padding around the swatches, but the larger padding around the current color spot feels more awkward now, with the same 2px radius looking extra out of place. The panel was originally designed to have that be edge-to-edge, like this:

Screenshot 2022-08-03 at 11 05 10

Screenshot 2022-08-03 at 11 05 14

I believe the previous non-standard padding is something we arrived at exactly because edge-to-edge turned out to be non-trivial to implement. Is that still the case or could we revisit that? CC: @ciampo in case you recall details.

@jameskoster
Copy link
Contributor Author

Oh yes, that would be a nice enhancement.

@ciampo
Copy link
Contributor

ciampo commented Aug 3, 2022

I believe the previous non-standard padding is something we arrived at exactly because edge-to-edge turned out to be non-trivial to implement. Is that still the case or could we revisit that? CC: @ciampo in case you recall details.

@mirka 's work with #42595 may make some of this easier, but we'd still need to take a closer look

@mirka
Copy link
Member

mirka commented Aug 4, 2022

@jasmussen I'm looking into getting the awkward padding resolved and also get the current color button be edge-to-edge, like in your mockup. One follow-up question: How do we want the edge-to-edge current color button to look when it's under the Solid/Gradient tab switcher? Edge-to-edge with no top-margin?

@jasmussen
Copy link
Contributor

Thank you!

One follow-up question: How do we want the edge-to-edge current color button to look when it's under the Solid/Gradient tab switcher? Edge-to-edge with no top-margin?

That's an excellent question. When there's no switcher, this is the previous flow mocked up:

Color ItemGroup Flow i5

I have an older discarded variant here that I don't love, which nevertheless affords an item above the edge to edge swatch:

Color ItemGroup Flow, i3

My preference, however, would be to always keep the current indicator as the first item, as seen in the mockups for this pattern addition:

152792450-44f41640-ca2a-4011-9309-bfdd09d028ae

That's one of the reasons why I've preferred the segmented control. It is secondary to the main color... a property, so to speak, rather than a distinct tab.

@jasmussen
Copy link
Contributor

Potentially also relevant, #39452 which details iterations to the duotone configuration, which also keeps the preview/current duotone at the top.

@mirka
Copy link
Member

mirka commented Aug 5, 2022

Started a tracking issue here: #43014

I love that design! Funnily, to me that flipped order makes the tab panel model even more "correct" 😆

I think the fundamental difference between a tab panel and a radio is that, a radio is for a "value" that is to be submitted or persisted, whereas a tab panel is just a temporary way to expand/collapse information, much like an accordion. In theory you should be able to expand all the panels into a vertical list and it should still make sense.

The flipped order you propose makes a very logical hierarchy to me:

  • Current color ("the value to be submitted")
  • Preset colors to choose from
    • Solid
      • Theme colors
      • Default colors
    • Gradient
      • Theme colors
      • Default colors

Whereas previously it was kind of murky:

  • Solid
    • Current color ("the value to be submitted")
    • Preset colors to choose from
      • Theme colors
      • Default colors
  • Gradient
    • Preset colors to choose from
      • Theme colors
      • Default colors
    • Current color ("the value to be submitted") 👈 this appears twice in the list when there's only one value that will be submitted from this popover

So that leads me to a few more questions:

  1. Am I correct to assume that we're also flipping the order for the Gradient picker? This part goes on top of the tabs switcher?

    The gradient swatch from the gradient panel
  2. Are we flipping the order when this picker appears in the Global Styles sidebar as well? (minus the edge-to-edge treatment maybe?)

@jasmussen
Copy link
Contributor

Am I correct to assume that we're also flipping the order for the Gradient picker? This part goes on top of the tabs switcher?

Good question. That could be interesting. It seems like that might also line up with some of the hierarchy proposed for gradients in the global styles inspector:
Screenshot 2022-08-05 at 10 39 39

Imprecise mockup, but something along these lines:

Frame 867

That would also answer your second question, as it appears the order in the Global Styles mockup I found already orders it current color/gradient first, followed by configuration, then swatches.

If you have a moment to look, @pablohoneyhoney, I'd love your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Colors Color management Needs Design Feedback Needs general design feedback. Needs Figma Update Needs an update to Figma for design purposes Needs User Documentation Needs new user documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants